Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
ffc9490a32 | ||
|
|
dc6625d33a | ||
|
|
dc8db159f9 | ||
|
|
86e7444603 | ||
|
|
2365806d18 | ||
|
|
7d348f243a | ||
|
|
ddd04623f7 | ||
|
|
9ad1477b1e |
@@ -3891,3 +3891,189 @@ scan takes a minute and is worth running before demoting anybody's links
|
|||||||
— `explore-album-details`'s tracklist (number / title / artist /
|
— `explore-album-details`'s tracklist (number / title / artist /
|
||||||
duration) is the one that plausibly *is* mostly link, and is the one
|
duration) is the one that plausibly *is* mostly link, and is the one
|
||||||
#5 is about to add selection to.
|
#5 is about to add selection to.
|
||||||
|
|
||||||
|
## A layout is still moving when a guard says it has arrived (measured 2026-08-20)
|
||||||
|
|
||||||
|
`album-dropdown.spec.ts` failed with `Expected 80, Received 10` twice
|
||||||
|
over two sessions, and #133 already strengthened its guard from
|
||||||
|
"scrollable at all" to "has at least the range the assertion needs".
|
||||||
|
That was necessary and could not be sufficient, and the reason is
|
||||||
|
structural rather than a matter of thresholds: **a guard and the write
|
||||||
|
it guards are separate CDP round trips**, so the page is free to
|
||||||
|
re-lay-out between them. Polling harder cannot close a window between
|
||||||
|
two moments; only removing the window can.
|
||||||
|
|
||||||
|
Measured directly, sampling `scrollHeight - clientHeight` on
|
||||||
|
`.grid-scroll-container` every frame across a 1440x900 → 900x600 resize,
|
||||||
|
three runs:
|
||||||
|
|
||||||
|
| t (ms) | range |
|
||||||
|
|---|---|
|
||||||
|
| 0 | 0 |
|
||||||
|
| 1 | **88** |
|
||||||
|
| 8–14 | 330 (settled) |
|
||||||
|
|
||||||
|
88 satisfies a guard asking for 80 and is not the settled value, so the
|
||||||
|
guard can pass while the grid is one layout pass from done. Under
|
||||||
|
full-suite load the transient is worse — the observed failure had 10 —
|
||||||
|
which is why it shows up on the second run of a suite and not in ten
|
||||||
|
consecutive runs of the file alone (0/10 both before and after the fix).
|
||||||
|
|
||||||
|
The shape to write instead: **one page-side call that performs the
|
||||||
|
action and returns what it observes**, with `expect.poll` retrying
|
||||||
|
*that*. `scrollTo()` sets `scrollTop` and returns `scrollTop`, so the
|
||||||
|
assertion is about what the grid did rather than about what it was
|
||||||
|
ready to do. `layout-overflow.spec.ts`'s sidebar probe already had the
|
||||||
|
fused half and was missing the retry; it has both now.
|
||||||
|
|
||||||
|
Worth generalising: a spec that resizes and then measures is asserting
|
||||||
|
about a moving target for the next dozen frames. Fuse, then poll.
|
||||||
|
|
||||||
|
## "The first N tracks" is not a way to ask for an ordinary one (2026-08-20)
|
||||||
|
|
||||||
|
`queue-selection.spec.ts` staged its queue from the first few rows of
|
||||||
|
`library.Library.GetTracks(0)` and clicked a track *name*, which
|
||||||
|
`explore-link` routes to that track's **album** page. Four tracks in the
|
||||||
|
fixture library have no album at all — `01 Tone A`, `02 Tone B`,
|
||||||
|
`Title Only`, `no-tags-at-all` — and a name with nothing to route to
|
||||||
|
renders as **plain text**, not as a link.
|
||||||
|
|
||||||
|
Two things follow, and the second is the sharper one.
|
||||||
|
|
||||||
|
**The order is the scan's.** `GetTracks` returns `audio_files.id` order,
|
||||||
|
i.e. the order the scan inserted rows, which depends on concurrency and
|
||||||
|
directory traversal. Locally the first eight are all from two proper
|
||||||
|
albums, so the spec passed twice over; CI rebuilds its seed with a real
|
||||||
|
scan, got a different eight, and failed on both engines. This is the
|
||||||
|
same family as "a seed freezes every default it has already persisted" —
|
||||||
|
the fixture library is not a list, it is a *set* with an incidental
|
||||||
|
order, and no spec should depend on that order.
|
||||||
|
|
||||||
|
**A loose locator hid it.** The row was located with
|
||||||
|
`.locator('.explore-link').first()`, and a row has two — the title and
|
||||||
|
the artist. When the title is plain text, `first()` silently resolves to
|
||||||
|
the **artist** link, so the click went somewhere real and the assertion
|
||||||
|
was about a destination the test had not exercised. `.track-title
|
||||||
|
.explore-link` is the locator that says which one it means; the loose
|
||||||
|
one turned a fixture problem into a mystery.
|
||||||
|
|
||||||
|
The general rule for this repo's fixture library: it is deliberately
|
||||||
|
full of edge cases (untagged, unicode, duplicates, extremes), so a spec
|
||||||
|
that wants an *ordinary* track has to **say so** — filter on the
|
||||||
|
property it depends on rather than slicing.
|
||||||
|
## A nested rule starting with an element name is dropped on the phone (2026-08-20)
|
||||||
|
|
||||||
|
`CLAUDE.md` records that the device renders in **Chrome 113**, which
|
||||||
|
does not have relaxed CSS nesting (Chrome 120). The consequence is
|
||||||
|
sharper than "some syntax is unavailable": a nested rule whose selector
|
||||||
|
begins with a bare identifier is not a parse error you would notice, it
|
||||||
|
is **silently dropped**.
|
||||||
|
|
||||||
|
Three such rules were live in `frontend/index.css`, all inside
|
||||||
|
`.bottom-bar`, and all therefore dead on the phone and only on the
|
||||||
|
phone:
|
||||||
|
|
||||||
|
```css
|
||||||
|
.bottom-bar {
|
||||||
|
#track-info { p { … } } /* the metadata's ellipsis */
|
||||||
|
now-playing { overflow: hidden; }
|
||||||
|
audio-player { margin: 0.5em 1em; }
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
The first is the interesting one: it is the *ellipsis* on the bottom
|
||||||
|
bar's track title and artist, so on the device that text has never
|
||||||
|
truncated — the same class of fault as `now-playing`'s marquee, whose
|
||||||
|
`text-overflow` sat on the wrong box and had never produced an ellipsis
|
||||||
|
in any mode. Both are invisible to every assertion and visible in a
|
||||||
|
screenshot.
|
||||||
|
|
||||||
|
`& p`, `& now-playing`, `& audio-player` are valid in both, so the fix
|
||||||
|
is one character per rule. What is worth keeping is the rule of thumb:
|
||||||
|
**inside a nested block, always write `&`** — and note that a rule
|
||||||
|
inside `@media` is *not* nested, so `@media … { bottom-nav { … } }`
|
||||||
|
elsewhere in that file is fine and needs nothing.
|
||||||
|
|
||||||
|
`make css-check` does not catch this (it looks for backticks that end a
|
||||||
|
tagged template early). Filed as an issue: the check is the natural
|
||||||
|
place for it, being the same shape of trap — a silent, phone-only,
|
||||||
|
screenshot-only failure.
|
||||||
|
|
||||||
|
## Centring a bar costs the control in the middle of it (measured 2026-08-20)
|
||||||
|
|
||||||
|
#23 asks for the transport centred in the bottom bar. The obvious
|
||||||
|
implementation — make the outer two grid tracks the same width, so the
|
||||||
|
middle is centred by construction — is right, and the first cut of it
|
||||||
|
was a regression, because "the same width" was taken to mean *the
|
||||||
|
metadata's* width on both sides.
|
||||||
|
|
||||||
|
Measured at 800px, with the seek bar's own track:
|
||||||
|
|
||||||
|
| layout | seek track | transport column |
|
||||||
|
|---|---|---|
|
||||||
|
| `320px 1fr auto` (before) | 257 | 407 |
|
||||||
|
| both sides `--now-playing-width` | **61** | 179 |
|
||||||
|
| both sides `min(--now-playing-width, 25%)` | 246 | 364 |
|
||||||
|
|
||||||
|
At 200% text the middle row is worse still: 130 before, **0** with the
|
||||||
|
uncapped sides. Centring is free at 1440 and expensive at 800, so a
|
||||||
|
change checked only at a comfortable width looks perfect.
|
||||||
|
|
||||||
|
The general form: **a symmetric layout reserves space on the side that
|
||||||
|
does not need it.** The right-hand group here is ~141px (volume plus
|
||||||
|
the queue button) and was being given 320 to keep the arithmetic
|
||||||
|
symmetric. Cap the side tracks against the *bar*, not against their
|
||||||
|
content, and the middle gets the difference.
|
||||||
|
|
||||||
|
The spec that pins this is two assertions, not one, and that split is
|
||||||
|
deliberate: an uncapped build is *perfectly centred* and fails only the
|
||||||
|
seek-bar width, so a spec asserting centring alone would have passed
|
||||||
|
the regression.
|
||||||
|
|
||||||
|
## The phone's way into Now Playing was under the artwork (measured 2026-08-20)
|
||||||
|
|
||||||
|
`phone-shell.spec.ts`'s "opens the full-screen now playing" failed in CI
|
||||||
|
on both engines, three times across two branches that could not have
|
||||||
|
caused it, and passed on re-run each time. It was filed as a flake
|
||||||
|
(#150). It is not one: **it depends on which track is playing.**
|
||||||
|
|
||||||
|
`.expand` — the phone's only route into `<now-playing-view>` — is
|
||||||
|
`position: absolute; inset: 0` inside `.cover-art-wrapper`, and
|
||||||
|
`.cover-art` is a **later sibling**. Both have `z-index: auto`, so they
|
||||||
|
tie on paint order and the later one wins. With an `<img>` that costs
|
||||||
|
nothing; with no artwork the placeholder `wa-icon` renders and takes
|
||||||
|
every click aimed at the button underneath it.
|
||||||
|
|
||||||
|
Measured at 390px with `elementFromPoint` at the button's centre:
|
||||||
|
|
||||||
|
| playing track | hit test |
|
||||||
|
|---|---|
|
||||||
|
| has artwork | `button.expand` |
|
||||||
|
| no artwork | **`wa-icon`** |
|
||||||
|
| no artwork, with `z-index: 1` | `button.expand` |
|
||||||
|
|
||||||
|
So on a phone, the only way into the full-screen player stopped working
|
||||||
|
whenever the current song had no cover — and this has nothing to do with
|
||||||
|
the fixture: any library has untagged files.
|
||||||
|
|
||||||
|
Three things worth keeping.
|
||||||
|
|
||||||
|
**"Flaky in CI" was the wrong diagnosis and it cost three cycles.** The
|
||||||
|
spec starts the *first* row of the track list, so which track it plays
|
||||||
|
is the order the scan inserted rows in — the same root cause as #156,
|
||||||
|
one spec over. A test whose subject is a hit test has to *choose* the
|
||||||
|
case that breaks it.
|
||||||
|
|
||||||
|
**The first two hypotheses were both wrong, and both were plausible.**
|
||||||
|
A custom element's upgrade replacing its own contents, and the cover
|
||||||
|
preview's `mouseenter` opening a popup under the pointer. Neither
|
||||||
|
survived contact with `elementFromPoint`, which took a minute and would
|
||||||
|
have saved the other two cycles.
|
||||||
|
|
||||||
|
**And the spec that pins it needs the 90-second track**, because a
|
||||||
|
2-second one finishes before the assertions run — the trap
|
||||||
|
`fixtures.ts` already documents. Note the filter that does *not* work:
|
||||||
|
`library.Track.CoverArt` is empty for all 31 fixture rows, so "the
|
||||||
|
first track with no cover art" selects nothing in particular. The
|
||||||
|
placeholder's presence is asserted instead, which is the property the
|
||||||
|
test actually depends on.
|
||||||
|
|||||||
@@ -1572,6 +1572,50 @@ And **the bar does not resize when a job starts**, which is the case the
|
|||||||
whole thing is for — a ResizeObserver on the header alone never fires,
|
whole thing is for — a ResizeObserver on the header alone never fires,
|
||||||
so every element child is observed too.
|
so every element child is observed too.
|
||||||
|
|
||||||
|
**The bottom bar is three columns whose outer two are the same width,
|
||||||
|
and that is what "centred" means.** It was `320px 1fr auto`, so the
|
||||||
|
transport sat in the middle of the space the metadata and the queue
|
||||||
|
button did not use — its centre was ~140px right of the window's at
|
||||||
|
every size (#23). The outer tracks are now the same expression, so the
|
||||||
|
middle one is centred by construction rather than by arithmetic that
|
||||||
|
has to be redone whenever a control joins the bar.
|
||||||
|
|
||||||
|
Four things about it are load-bearing.
|
||||||
|
|
||||||
|
**The side width is the metadata's, capped at a quarter of the bar**,
|
||||||
|
and the cap is not tidiness — it was measured as a regression first.
|
||||||
|
Reserving the full `--now-playing-width` on *both* sides costs the
|
||||||
|
transport twice: at 800px the outer pair wanted 640 of 800 and the seek
|
||||||
|
bar's track fell from **257px to 61px**, and to 0 at 200% text. The
|
||||||
|
control you drag was being squeezed to centre the buttons above it.
|
||||||
|
With the cap it is 246px at 800, which is parity with the uncentred
|
||||||
|
layout.
|
||||||
|
|
||||||
|
**The cap is a `min()` rather than a breakpoint** because
|
||||||
|
`--now-playing-width` is *user state* — the metadata panel has a drag
|
||||||
|
handle — and the same reasoning the queue panel's overlay mode uses
|
||||||
|
applies: a rule that assumed the default 320 would be wrong by whatever
|
||||||
|
the user dragged. Tying both sides to that variable is also what keeps
|
||||||
|
the handle meaningful; a plain `1fr … 1fr` would centre the transport
|
||||||
|
just as well and silently make dragging a no-op.
|
||||||
|
|
||||||
|
**The volume moved out of `audio-player` and into the bar** (#42),
|
||||||
|
because the transport column has to hold the transport and nothing
|
||||||
|
else or "centred" means centred with a slider bolted to one side. It
|
||||||
|
lives in `.bar-end` with the queue button — one cell, not two columns,
|
||||||
|
since the centring compares *columns* and a separate volume track would
|
||||||
|
make the outer pair unequal by whatever the slider measures.
|
||||||
|
|
||||||
|
And **the slider is inline by default, with the popup as a setting**
|
||||||
|
whose stored flag names the *popup*: `backend/config`'s polarity rule,
|
||||||
|
where the zero value has to be the intended answer, so an existing
|
||||||
|
`config.toml` with no key gets the new default without a migration.
|
||||||
|
Inline, the icon becomes the mute toggle and is named after that action
|
||||||
|
rather than after the state, because with the slider beside it there is
|
||||||
|
nothing left to disclose. It stands down below 600px whatever the
|
||||||
|
setting says — that is about the platform rather than preference, and
|
||||||
|
is why `mediacontrols`' Android handler implements no volume callback.
|
||||||
|
|
||||||
**900 is the worst desktop width, not the 800×600 minimum.** The
|
**900 is the worst desktop width, not the 800×600 minimum.** The
|
||||||
sidebar collapses to icons *below* 900, so the main panel is 843px at
|
sidebar collapses to icons *below* 900, so the main panel is 843px at
|
||||||
899 and 700px at 900 — the narrowest content area any desktop width
|
899 and 700px at 900 — the narrowest content area any desktop width
|
||||||
|
|||||||
@@ -666,6 +666,49 @@ func (c *Config) SetAllowMeteredCatalogDownload(allow bool) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// GetPopupVolume reports whether the bottom bar's volume control is a
|
||||||
|
// click-to-open popup rather than an inline slider (#42).
|
||||||
|
func (c *Config) GetPopupVolume() bool {
|
||||||
|
if c.General == nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
return c.General.PopupVolume
|
||||||
|
}
|
||||||
|
|
||||||
|
// SetPopupVolume saves the volume control's presentation.
|
||||||
|
//
|
||||||
|
// Nothing to validate: both values are legal at every width, and the
|
||||||
|
// frontend additionally stands the inline slider down below the phone
|
||||||
|
// breakpoint whatever this says, because that is about room rather than
|
||||||
|
// about preference.
|
||||||
|
func (c *Config) SetPopupVolume(popup bool) error {
|
||||||
|
if c.General == nil {
|
||||||
|
c.General = &GeneralConfig{}
|
||||||
|
c.General.ApplyDefaults()
|
||||||
|
}
|
||||||
|
|
||||||
|
c.General.PopupVolume = popup
|
||||||
|
|
||||||
|
if err := c.Save(); err != nil {
|
||||||
|
return fmt.Errorf(
|
||||||
|
"could not save config: %w", err,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
events.Emit(
|
||||||
|
c.ctx,
|
||||||
|
events.GeneralConfigChanged,
|
||||||
|
map[string]any{
|
||||||
|
"PopupVolume": popup,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
c.logger.Info("volume control presentation updated", "popup", popup)
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
// GetViewVisibility reports which primary views the sidebar should
|
// GetViewVisibility reports which primary views the sidebar should
|
||||||
// show, answered for every known view rather than only the ones the
|
// show, answered for every known view rather than only the ones the
|
||||||
// config mentions -- so the frontend filters on a value and never has
|
// config mentions -- so the frontend filters on a value and never has
|
||||||
|
|||||||
@@ -188,3 +188,43 @@ func TestEmit_FavoritesChangeCarriesFullConfig(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestEmit_PopupVolumeRoundTripsAndDefaultsToInline pins both halves of
|
||||||
|
// #42's storage decision.
|
||||||
|
//
|
||||||
|
// The **default** is the load-bearing one: inline is what a fresh
|
||||||
|
// install and an existing `config.toml` with no such key must both
|
||||||
|
// produce, which is why the field names the popup rather than the
|
||||||
|
// inline slider. A flag spelled the other way round would default to
|
||||||
|
// false, hand every existing install the popup this issue exists to
|
||||||
|
// stop being the only option, and need a migration to say otherwise.
|
||||||
|
func TestEmit_PopupVolumeRoundTripsAndDefaultsToInline(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
conf, rec := setupRecordedConfig(t)
|
||||||
|
|
||||||
|
if conf.GetPopupVolume() {
|
||||||
|
t.Error("a config with no PopupVolume key wants the popup, want inline")
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := conf.SetPopupVolume(true); err != nil {
|
||||||
|
t.Fatalf("SetPopupVolume: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if !conf.GetPopupVolume() {
|
||||||
|
t.Error("GetPopupVolume = false after setting it true")
|
||||||
|
}
|
||||||
|
|
||||||
|
data := payloadMap(t, rec, events.GeneralConfigChanged)
|
||||||
|
if data["PopupVolume"] != true {
|
||||||
|
t.Errorf("PopupVolume = %v, want true", data["PopupVolume"])
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := conf.SetPopupVolume(false); err != nil {
|
||||||
|
t.Fatalf("SetPopupVolume(false): %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if conf.GetPopupVolume() {
|
||||||
|
t.Error("GetPopupVolume = true after setting it false")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -54,6 +54,16 @@ type GeneralConfig struct {
|
|||||||
// so an existing config with no such key refuses by default rather
|
// so an existing config with no such key refuses by default rather
|
||||||
// than needing a migration to become careful.
|
// than needing a migration to become careful.
|
||||||
AllowMeteredCatalogDownload bool `toml:"AllowMeteredCatalogDownload"`
|
AllowMeteredCatalogDownload bool `toml:"AllowMeteredCatalogDownload"`
|
||||||
|
// PopupVolume draws the bottom bar's volume as a click-to-open popup
|
||||||
|
// instead of a slider that is always there (#42).
|
||||||
|
//
|
||||||
|
// The polarity is the rule this file already states twice: **the
|
||||||
|
// zero value is the intended answer**. Inline is the new default, so
|
||||||
|
// the flag has to name the *other* choice — an `InlineVolume bool`
|
||||||
|
// would default to false and give every existing install the popup
|
||||||
|
// this issue exists to stop being the only option, and would need a
|
||||||
|
// migration to say otherwise.
|
||||||
|
PopupVolume bool `toml:"PopupVolume"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// ApplyDefaults fills zero-value fields with sensible defaults.
|
// ApplyDefaults fills zero-value fields with sensible defaults.
|
||||||
|
|||||||
@@ -110,27 +110,30 @@ test.describe('the album dropdown', () => {
|
|||||||
await app.setViewportSize({ width: 900, height: 600 });
|
await app.setViewportSize({ width: 900, height: 600 });
|
||||||
|
|
||||||
try {
|
try {
|
||||||
// Wait for the range the assertion below actually needs, not for
|
// The container has to be a scroller at all, which is the thing
|
||||||
// "scrollable at all" (#133). The guard used to be
|
// the defect behind this spec broke and is a property rather
|
||||||
// `scrollHeight > clientHeight + 40` while the next line asks to
|
// than a moment.
|
||||||
// reach 80, so any range in 41-79 satisfied it and could not
|
|
||||||
// satisfy the assertion — and the grid passes through exactly
|
|
||||||
// that while it settles, because it recomputes its columns after
|
|
||||||
// the resize rather than during it. The settled range here is
|
|
||||||
// 330, so this waits rather than weakening anything.
|
|
||||||
await expect
|
await expect
|
||||||
.poll(() => scrollRange(app))
|
.poll(() => scrollRange(app))
|
||||||
.toMatchObject({ room: true, overflowY: 'auto' });
|
.toMatchObject({ overflowY: 'auto' });
|
||||||
|
|
||||||
await app.evaluate((target) => {
|
// **Scrolling it and reading it back are one round trip** (#151).
|
||||||
const sc = document
|
//
|
||||||
.querySelector('cover-grid')
|
// #133 made the guard ask for the range this needs rather than
|
||||||
?.shadowRoot?.querySelector('.grid-scroll-container');
|
// for "scrollable at all", which was necessary and is not
|
||||||
|
// sufficient: a guard and the write it guards are separate
|
||||||
if (sc) sc.scrollTop = target;
|
// `evaluate` calls, so the grid can satisfy the range and settle
|
||||||
}, SCROLL_TARGET);
|
// out of it before the write lands. It still does — observed as
|
||||||
|
// `Expected 80, Received 10` in the second of three consecutive
|
||||||
expect(await scrollTop(app)).toBe(SCROLL_TARGET);
|
// full-suite runs, with the spec green alone on the same app
|
||||||
|
// straight afterwards.
|
||||||
|
//
|
||||||
|
// Polling harder cannot close a window between two moments; only
|
||||||
|
// removing the window can. So the probe sets `scrollTop` and
|
||||||
|
// returns what it reads back, in one page-side call, and the
|
||||||
|
// poll retries *that* — which also means the assertion is about
|
||||||
|
// what the grid did rather than about what it was ready to do.
|
||||||
|
await expect.poll(() => scrollTo(app, SCROLL_TARGET)).toBe(SCROLL_TARGET);
|
||||||
|
|
||||||
// And the dropdown it opens is on screen, wherever the manager
|
// And the dropdown it opens is on screen, wherever the manager
|
||||||
// decides that leaves the scroll. It is *not* "the position is
|
// decides that leaves the scroll. It is *not* "the position is
|
||||||
@@ -261,7 +264,7 @@ async function closeDropdown(app: Page): Promise<void> {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Whether the grid can scroll at all, which decides if a probe can move. */
|
/** Whether the grid is a scroller at all, which is what the bug broke. */
|
||||||
async function scrollRange(app: Page) {
|
async function scrollRange(app: Page) {
|
||||||
return app.evaluate((target) => {
|
return app.evaluate((target) => {
|
||||||
const sc = document
|
const sc = document
|
||||||
@@ -269,22 +272,37 @@ async function scrollRange(app: Page) {
|
|||||||
?.shadowRoot?.querySelector('.grid-scroll-container');
|
?.shadowRoot?.querySelector('.grid-scroll-container');
|
||||||
|
|
||||||
return {
|
return {
|
||||||
// `room` is the precondition of the assertion that follows it:
|
// Reported for the failure message rather than waited on: `room`
|
||||||
// enough range to actually reach the target. A threshold below
|
// was the guard #133 strengthened, and #151 is that a guard in
|
||||||
// what the caller depends on is not a guard.
|
// its own round trip cannot speak for the write in the next one.
|
||||||
|
// `scrollTo` below is the assertion now; this says *why* it did
|
||||||
|
// not reach the target when it does not.
|
||||||
room: !!sc && sc.scrollHeight - sc.clientHeight >= target,
|
room: !!sc && sc.scrollHeight - sc.clientHeight >= target,
|
||||||
overflowY: sc ? getComputedStyle(sc).overflowY : '',
|
overflowY: sc ? getComputedStyle(sc).overflowY : '',
|
||||||
};
|
};
|
||||||
}, SCROLL_TARGET);
|
}, SCROLL_TARGET);
|
||||||
}
|
}
|
||||||
|
|
||||||
async function scrollTop(app: Page): Promise<number> {
|
/**
|
||||||
return app.evaluate(
|
* Scroll the grid and report where it actually landed, in one call.
|
||||||
() =>
|
*
|
||||||
document
|
* The whole point is that the set and the read share a moment: a
|
||||||
.querySelector('cover-grid')
|
* `scrollTop` write is clamped to the range *at the instant it lands*,
|
||||||
?.shadowRoot?.querySelector('.grid-scroll-container')?.scrollTop ?? -1,
|
* so reading it back in a second round trip asks a container that may
|
||||||
);
|
* have re-laid out in between.
|
||||||
|
*/
|
||||||
|
async function scrollTo(app: Page, target: number): Promise<number> {
|
||||||
|
return app.evaluate((to) => {
|
||||||
|
const sc = document
|
||||||
|
.querySelector('cover-grid')
|
||||||
|
?.shadowRoot?.querySelector('.grid-scroll-container');
|
||||||
|
|
||||||
|
if (!sc) return -1;
|
||||||
|
|
||||||
|
sc.scrollTop = to;
|
||||||
|
|
||||||
|
return sc.scrollTop;
|
||||||
|
}, target);
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Whether the open dropdown is inside the scroll container's viewport. */
|
/** Whether the open dropdown is inside the scroll container's viewport. */
|
||||||
|
|||||||
@@ -0,0 +1,147 @@
|
|||||||
|
import { test, expect, callBinding, NO_QUEUE_SOURCE } from '../support/fixtures.js';
|
||||||
|
import type { Page } from '@playwright/test';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The bottom bar's two promises (#23, #42): the transport is centred in
|
||||||
|
* the window, and the volume is a slider rather than a popup.
|
||||||
|
*
|
||||||
|
* **"Centred" is measured against the window, not against the space
|
||||||
|
* left over**, which is the whole of #23. The bar was
|
||||||
|
* `320px 1fr auto`, so the transport sat in the middle of what the
|
||||||
|
* metadata and the queue button did not use — its centre was ~140px
|
||||||
|
* right of the window's at every size, which reads as an alignment
|
||||||
|
* mistake rather than as a layout choice.
|
||||||
|
*
|
||||||
|
* The mechanism is that the outer two columns are the same width, so
|
||||||
|
* this asserts the *outcome* (centre lines up) rather than the CSS. A
|
||||||
|
* spec that checked `grid-template-columns` would pass on any build
|
||||||
|
* that kept the declaration and broke the result.
|
||||||
|
*/
|
||||||
|
|
||||||
|
/** Where the transport sits, against where the window's centre is. */
|
||||||
|
const geometry = (app: Page) =>
|
||||||
|
app.evaluate(() => {
|
||||||
|
const bar = document.querySelector<HTMLElement>('.bottom-bar')!;
|
||||||
|
const player = document.querySelector<HTMLElement>('audio-player')!;
|
||||||
|
const b = bar.getBoundingClientRect();
|
||||||
|
const p = player.getBoundingClientRect();
|
||||||
|
|
||||||
|
const seek = player.shadowRoot
|
||||||
|
?.querySelector('seek-bar')
|
||||||
|
?.shadowRoot?.querySelector('wa-slider');
|
||||||
|
|
||||||
|
return {
|
||||||
|
offset: Math.round(p.left + p.width / 2 - (b.left + b.width / 2)),
|
||||||
|
barHeight: Math.round(b.height),
|
||||||
|
seekWidth: seek ? Math.round(seek.getBoundingClientRect().width) : -1,
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
/** Something has to be playing before the transport draws a seek bar. */
|
||||||
|
async function play(app: Page): Promise<void> {
|
||||||
|
const paths = await app.evaluate(async () => {
|
||||||
|
const tracks = (await window.__yjEvents.call(
|
||||||
|
'library.Library.GetTracks',
|
||||||
|
[0],
|
||||||
|
10_000,
|
||||||
|
)) as { FilePath: string }[];
|
||||||
|
|
||||||
|
return tracks.slice(0, 3).map((t) => t.FilePath);
|
||||||
|
});
|
||||||
|
|
||||||
|
await callBinding(app, 'queue.Queue.SetQueue', [
|
||||||
|
paths,
|
||||||
|
0,
|
||||||
|
false,
|
||||||
|
NO_QUEUE_SOURCE,
|
||||||
|
]);
|
||||||
|
await callBinding(app, 'queue.Queue.Play');
|
||||||
|
await expect(app.getByTestId('now-playing-title')).not.toBeEmpty();
|
||||||
|
}
|
||||||
|
|
||||||
|
test.describe('the bottom bar', () => {
|
||||||
|
test.afterEach(async ({ app }) => {
|
||||||
|
await callBinding(app, 'queue.Queue.Clear').catch(() => {
|
||||||
|
/* already empty */
|
||||||
|
});
|
||||||
|
await app.setViewportSize({ width: 1440, height: 900 });
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Four widths, because a centring bug is a function of width: the old
|
||||||
|
* layout was off by half the difference between the two outer
|
||||||
|
* columns, so it was wrong by a different amount at each one and
|
||||||
|
* exactly right at none.
|
||||||
|
*/
|
||||||
|
for (const width of [800, 900, 1100, 1440]) {
|
||||||
|
test(`centres the transport in the window at ${width}px`, async ({
|
||||||
|
app,
|
||||||
|
}) => {
|
||||||
|
await app.setViewportSize({ width, height: 700 });
|
||||||
|
await play(app);
|
||||||
|
|
||||||
|
await expect.poll(() => geometry(app).then((g) => g.offset)).toBe(0);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The seek bar is what the centring is *paid for* with, so it is
|
||||||
|
* asserted rather than assumed.
|
||||||
|
*
|
||||||
|
* Reserving the metadata's full width on both sides centres the
|
||||||
|
* transport perfectly and squeezes the control you drag: measured
|
||||||
|
* during this work at **61px of track at 800px**, against 257 before
|
||||||
|
* the change. The side columns are capped at a quarter of the bar for
|
||||||
|
* that reason, and this is the number that says so — 246 at 800px,
|
||||||
|
* which is parity with the uncentred layout.
|
||||||
|
*/
|
||||||
|
test('does not pay for the centring with the seek bar', async ({ app }) => {
|
||||||
|
await app.setViewportSize({ width: 800, height: 700 });
|
||||||
|
await play(app);
|
||||||
|
|
||||||
|
await expect
|
||||||
|
.poll(() => geometry(app).then((g) => g.seekWidth))
|
||||||
|
.toBeGreaterThan(200);
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* #42: the slider is simply there. Three gestures — click open, drag,
|
||||||
|
* click closed — is what a bottom bar has room not to ask for.
|
||||||
|
*/
|
||||||
|
test('shows the volume slider without a click', async ({ app }) => {
|
||||||
|
await app.setViewportSize({ width: 1440, height: 900 });
|
||||||
|
|
||||||
|
const volume = app.locator('.bottom-bar volume-control');
|
||||||
|
|
||||||
|
await expect(volume).toBeVisible();
|
||||||
|
await expect(volume.locator('wa-slider')).toBeVisible();
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* And the inline icon is the mute toggle, because with the slider
|
||||||
|
* beside it there is nothing left to disclose. The name follows the
|
||||||
|
* action rather than the state for the same reason.
|
||||||
|
*/
|
||||||
|
test('names the inline icon after what it does', async ({ app }) => {
|
||||||
|
await app.setViewportSize({ width: 1440, height: 900 });
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
app.locator('.bottom-bar volume-control').getByRole('button', {
|
||||||
|
name: 'Mute',
|
||||||
|
}),
|
||||||
|
).toBeVisible();
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The bar is a fixed 4em row and the transport sits in it. A slider
|
||||||
|
* with a label grows `#slider` by 8px unless `wa-slider-label.css`
|
||||||
|
* suppresses it, which moved the whole bar the last time — so the
|
||||||
|
* height is pinned here rather than left to a screenshot.
|
||||||
|
*/
|
||||||
|
test('stays 4em tall', async ({ app }) => {
|
||||||
|
await app.setViewportSize({ width: 1440, height: 900 });
|
||||||
|
await play(app);
|
||||||
|
|
||||||
|
await expect.poll(() => geometry(app).then((g) => g.barHeight)).toBe(64);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -29,18 +29,13 @@ test.describe('a control says what it controls', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
test('the volume slider is announced as Volume', async ({ app }) => {
|
test('the volume slider is announced as Volume', async ({ app }) => {
|
||||||
// The popup renders no slider at all while closed, the same way the
|
// No disclosure to open first, and no state to put back afterwards:
|
||||||
// queue panel renders no list — so this has to open it first.
|
// #42 made the slider inline, so it is simply there. The assertion
|
||||||
await app.getByRole('button', { name: /volume/i }).click();
|
// is unchanged — the *name* is the subject here, and the route to
|
||||||
|
// the control got shorter rather than different.
|
||||||
await expect(
|
await expect(
|
||||||
app.getByRole('slider', { name: 'Volume' }),
|
app.getByRole('slider', { name: 'Volume' }),
|
||||||
).toBeVisible();
|
).toBeVisible();
|
||||||
|
|
||||||
// Leave the transport as it was found: the specs share one page in
|
|
||||||
// file order, and an open popup covers the buttons beneath it.
|
|
||||||
await app.keyboard.press('Escape');
|
|
||||||
await app.locator('body').click({ position: { x: 5, y: 5 } });
|
|
||||||
});
|
});
|
||||||
|
|
||||||
test('naming the slider did not move the transport', async ({ app }) => {
|
test('naming the slider did not move the transport', async ({ app }) => {
|
||||||
|
|||||||
@@ -132,22 +132,34 @@ test.describe('the app fits in its own window', () => {
|
|||||||
// be dragged here, but a scaled display or a large system font can
|
// be dragged here, but a scaled display or a large system font can
|
||||||
// still land the layout in it, and clipping the nav with no scroll
|
// still land the layout in it, and clipping the nav with no scroll
|
||||||
// is the failure that made Settings unreachable.
|
// is the failure that made Settings unreachable.
|
||||||
const reachable = await app.locator('app-sidebar').evaluate((el) => {
|
//
|
||||||
const settings = el.shadowRoot?.querySelector<HTMLElement>(
|
// The scroll and the measurement share one `evaluate` — #151's
|
||||||
'[data-testid="nav-settings"]',
|
// rule, which this already had — and the whole probe is polled,
|
||||||
);
|
// which it did not: a viewport change settles asynchronously, so a
|
||||||
|
// single attempt reads whatever the sidebar happened to be doing.
|
||||||
|
// The probe is safe to repeat because scrolling to the bottom twice
|
||||||
|
// is scrolling to the bottom.
|
||||||
|
await expect
|
||||||
|
.poll(() =>
|
||||||
|
app.locator('app-sidebar').evaluate((el) => {
|
||||||
|
const settings = el.shadowRoot?.querySelector<HTMLElement>(
|
||||||
|
'[data-testid="nav-settings"]',
|
||||||
|
);
|
||||||
|
|
||||||
if (!settings) return null;
|
if (!settings) return null;
|
||||||
|
|
||||||
el.scrollTop = el.scrollHeight;
|
el.scrollTop = el.scrollHeight;
|
||||||
|
|
||||||
const item = settings.getBoundingClientRect();
|
const item = settings.getBoundingClientRect();
|
||||||
const pane = el.getBoundingClientRect();
|
const pane = el.getBoundingClientRect();
|
||||||
|
|
||||||
return item.bottom <= Math.ceil(pane.bottom) && item.top >= Math.floor(pane.top);
|
return (
|
||||||
});
|
item.bottom <= Math.ceil(pane.bottom) &&
|
||||||
|
item.top >= Math.floor(pane.top)
|
||||||
expect(reachable).toBe(true);
|
);
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
.toBe(true);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { test, expect } from '../support/fixtures.js';
|
import { test, expect, LONG_TRACK } from '../support/fixtures.js';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The phone shell (plan 016 B2, phase 1).
|
* The phone shell (plan 016 B2, phase 1).
|
||||||
@@ -138,6 +138,78 @@ test.describe('the shell on a phone', () => {
|
|||||||
.toHaveAttribute('data-active-view', 'tracks');
|
.toHaveAttribute('data-active-view', 'tracks');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The same journey with a track that has **no cover art** (#150).
|
||||||
|
*
|
||||||
|
* The test above starts the *first* row of the track list, so which
|
||||||
|
* track it plays is the order the scan inserted them in — and the
|
||||||
|
* answer decided whether it passed. A track with artwork renders an
|
||||||
|
* `<img>`, which is no obstacle; one without renders a placeholder
|
||||||
|
* `wa-icon`, which took every click aimed at the button beneath it,
|
||||||
|
* because that button is absolutely positioned with `z-index: auto`
|
||||||
|
* and the art is a *later* sibling. They tied, and the later one won.
|
||||||
|
*
|
||||||
|
* So this picks a track *for* the property that broke it, which is
|
||||||
|
* the only way the assertion means anything: the version above passes
|
||||||
|
* on a broken build roughly two runs in three, which is exactly how
|
||||||
|
* it came to cost three CI cycles across two branches that could not
|
||||||
|
* have caused it.
|
||||||
|
*/
|
||||||
|
test('opens the full-screen now playing for a track with no art', async ({
|
||||||
|
app,
|
||||||
|
}) => {
|
||||||
|
// `LONG_TRACK` by name, and not "the first track with no
|
||||||
|
// CoverArt": the *library* model reports that field empty for
|
||||||
|
// every row in this fixture (31 of 31), so filtering on it selects
|
||||||
|
// nothing in particular and picked a 2-second track, which had
|
||||||
|
// finished before the assertions ran. The placeholder check below
|
||||||
|
// is what actually holds the property this test needs.
|
||||||
|
const started = await app.evaluate(async (longTitle) => {
|
||||||
|
const tracks = (await window.__yjEvents.call(
|
||||||
|
'library.Library.GetTracks',
|
||||||
|
[0],
|
||||||
|
10_000,
|
||||||
|
)) as { FilePath: string; TrackName: string }[];
|
||||||
|
|
||||||
|
const bare = tracks.find((t) => t.TrackName === longTitle);
|
||||||
|
|
||||||
|
if (!bare) return null;
|
||||||
|
|
||||||
|
await window.__yjEvents.call(
|
||||||
|
'queue.Queue.SetQueue',
|
||||||
|
[[bare.FilePath], 0, false, { type: '', id: 0, label: '' }],
|
||||||
|
10_000,
|
||||||
|
);
|
||||||
|
await window.__yjEvents.call('queue.Queue.Play', [], 5_000);
|
||||||
|
|
||||||
|
return bare.TrackName;
|
||||||
|
}, LONG_TRACK);
|
||||||
|
|
||||||
|
expect(started).toBe(LONG_TRACK);
|
||||||
|
|
||||||
|
await expect(app.getByTestId('now-playing-title')).not.toBeEmpty();
|
||||||
|
|
||||||
|
// **The placeholder is the whole point**, so it is asserted rather
|
||||||
|
// than assumed: this test is about the thing that renders when
|
||||||
|
// there is no artwork. If the fixture ever gives this album a
|
||||||
|
// cover, this fails and says so instead of passing while measuring
|
||||||
|
// the easy case.
|
||||||
|
//
|
||||||
|
// One selector rather than a chain from the host: Playwright's CSS
|
||||||
|
// engine pierces an open shadow root, and chaining from the host
|
||||||
|
// element does not reach into it.
|
||||||
|
await expect(
|
||||||
|
app.locator('now-playing .cover-placeholder'),
|
||||||
|
).toBeAttached();
|
||||||
|
|
||||||
|
await app.getByTestId('open-now-playing').click();
|
||||||
|
|
||||||
|
await expect(app.getByTestId('main-content'))
|
||||||
|
.toHaveAttribute('data-active-view', 'now-playing');
|
||||||
|
|
||||||
|
await app.getByTestId('npv-back').click();
|
||||||
|
});
|
||||||
|
|
||||||
test('offers no way in on a desktop, where the bar is whole', async ({ app }) => {
|
test('offers no way in on a desktop, where the bar is whole', async ({ app }) => {
|
||||||
await app.setViewportSize({ width: 1440, height: 900 });
|
await app.setViewportSize({ width: 1440, height: 900 });
|
||||||
|
|
||||||
@@ -154,9 +226,26 @@ test.describe('the shell on a phone', () => {
|
|||||||
await expect(app.locator('now-playing')).toBeVisible();
|
await expect(app.locator('now-playing')).toBeVisible();
|
||||||
|
|
||||||
// Volume is the hardware keys' job on a phone, and a 4px seek bar
|
// Volume is the hardware keys' job on a phone, and a 4px seek bar
|
||||||
// is not a thumb target -- both belong to a later phase's
|
// is not a thumb target -- both belong to the full-screen
|
||||||
// full-screen now-playing view.
|
// now-playing view.
|
||||||
await expect(app.locator('audio-player volume-control')).toBeHidden();
|
//
|
||||||
|
// `.bottom-bar volume-control`, not `audio-player volume-control`:
|
||||||
|
// #42 moved the control out of that component and into the bar, and
|
||||||
|
// **the old locator would have kept passing** — `toBeHidden()` is
|
||||||
|
// satisfied by an element that does not exist, so this assertion
|
||||||
|
// would have gone on reporting success about nothing. Its partner
|
||||||
|
// below is what makes this one mean something.
|
||||||
|
await expect(app.locator('.bottom-bar volume-control')).toBeHidden();
|
||||||
|
|
||||||
|
// The element is there and hidden, rather than absent: the check
|
||||||
|
// above cannot tell those apart on its own.
|
||||||
|
await expect(app.locator('.bottom-bar volume-control')).toHaveCount(1);
|
||||||
|
|
||||||
|
// And the seek bar is still inside the transport, where it stands
|
||||||
|
// down by its own media query.
|
||||||
|
await expect(
|
||||||
|
app.locator('audio-player').locator('seek-bar'),
|
||||||
|
).toBeHidden();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -93,10 +93,31 @@ async function queueSixAndOpen(app: Page): Promise<void> {
|
|||||||
'library.Library.GetTracks',
|
'library.Library.GetTracks',
|
||||||
[0],
|
[0],
|
||||||
10_000,
|
10_000,
|
||||||
)) as { FilePath: string; TrackName: string }[];
|
)) as { FilePath: string; TrackName: string; Album: string }[];
|
||||||
|
|
||||||
const long = tracks.find((t) => t.TrackName === longTitle);
|
const long = tracks.find((t) => t.TrackName === longTitle);
|
||||||
const rest = tracks.filter((t) => t.TrackName !== longTitle).slice(0, 5);
|
|
||||||
|
/**
|
||||||
|
* **Tracks that have an album**, which is a requirement of one of
|
||||||
|
* the tests and was previously left to luck (#156).
|
||||||
|
*
|
||||||
|
* `explore-link` routes a track name to its *album's* page, so a
|
||||||
|
* track with no album renders a name that navigates nowhere — and
|
||||||
|
* the fixture library deliberately contains two (`01 Tone A`,
|
||||||
|
* `02 Tone B`). Which tracks arrive first is `audio_files.id`
|
||||||
|
* order, i.e. the order the **scan** inserted them, which depends
|
||||||
|
* on concurrency and directory traversal: locally the first eight
|
||||||
|
* all had albums and the spec passed twice over, and CI rebuilds
|
||||||
|
* its seed with a real scan and got a different eight.
|
||||||
|
*
|
||||||
|
* Asking for what the test needs is the fix. It is not a
|
||||||
|
* narrowing: every assertion here wants an ordinary track, and
|
||||||
|
* "the first five rows" was never a way to ask for one in a
|
||||||
|
* library whose whole purpose is edge cases.
|
||||||
|
*/
|
||||||
|
const rest = tracks
|
||||||
|
.filter((t) => t.TrackName !== longTitle && t.Album !== '')
|
||||||
|
.slice(0, 5);
|
||||||
|
|
||||||
// Index 3 is the long one: far enough down that a shift-extend has
|
// Index 3 is the long one: far enough down that a shift-extend has
|
||||||
// room either side of it.
|
// room either side of it.
|
||||||
@@ -223,7 +244,14 @@ test.describe('selecting in the queue with a mouse', () => {
|
|||||||
.poll(() => selected(app), { timeout: HIGHLIGHT_MS })
|
.poll(() => selected(app), { timeout: HIGHLIGHT_MS })
|
||||||
.toEqual([1]);
|
.toEqual([1]);
|
||||||
|
|
||||||
await row(app, 2).locator('.explore-link').first().click();
|
// `.track-title .explore-link`, not `.explore-link` first(): a row
|
||||||
|
// has two, and which one `first()` finds depends on whether the
|
||||||
|
// *title* is a link at all. It is not, for a track with no album —
|
||||||
|
// `explore-link` renders plain text where it cannot route — so the
|
||||||
|
// loose locator silently clicked the **artist** instead and the
|
||||||
|
// assertion below was about a different destination than the one
|
||||||
|
// being exercised (#156).
|
||||||
|
await row(app, 2).locator('.track-title .explore-link').click();
|
||||||
|
|
||||||
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||||
'data-active-view',
|
'data-active-view',
|
||||||
|
|||||||
@@ -69,6 +69,14 @@ export function GetPinDefaultPlaylist(): $CancellablePromise<boolean> {
|
|||||||
return $Call.ByID(3818283301);
|
return $Call.ByID(3818283301);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* GetPopupVolume reports whether the bottom bar's volume control is a
|
||||||
|
* click-to-open popup rather than an inline slider (#42).
|
||||||
|
*/
|
||||||
|
export function GetPopupVolume(): $CancellablePromise<boolean> {
|
||||||
|
return $Call.ByID(2885777);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* GetQueueFallback returns what plays, if anything, once the queue
|
* GetQueueFallback returns what plays, if anything, once the queue
|
||||||
* runs out.
|
* runs out.
|
||||||
@@ -207,6 +215,18 @@ export function SetPinDefaultPlaylist(pin: boolean): $CancellablePromise<void> {
|
|||||||
return $Call.ByID(372446849, pin);
|
return $Call.ByID(372446849, pin);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* SetPopupVolume saves the volume control's presentation.
|
||||||
|
*
|
||||||
|
* Nothing to validate: both values are legal at every width, and the
|
||||||
|
* frontend additionally stands the inline slider down below the phone
|
||||||
|
* breakpoint whatever this says, because that is about room rather than
|
||||||
|
* about preference.
|
||||||
|
*/
|
||||||
|
export function SetPopupVolume(popup: boolean): $CancellablePromise<void> {
|
||||||
|
return $Call.ByID(1430308453, popup);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* SetQueueFallback validates and saves a new queue-fallback mode.
|
* SetQueueFallback validates and saves a new queue-fallback mode.
|
||||||
*/
|
*/
|
||||||
|
|||||||
+78
-5
@@ -211,7 +211,39 @@ body div.sidebar {
|
|||||||
padding: 0.25em;
|
padding: 0.25em;
|
||||||
background-color: var(--yj-bg-elevated, #343a40);
|
background-color: var(--yj-bg-elevated, #343a40);
|
||||||
display: grid;
|
display: grid;
|
||||||
grid-template-columns: var(--now-playing-width, 320px) 1fr auto;
|
/* Three columns whose outer two are the *same* width, which is what
|
||||||
|
centres the middle one (#23). It was `var(--now-playing-width) 1fr
|
||||||
|
auto`, so the transport's centre sat at `W/2 + 140px` — in the
|
||||||
|
middle of the space left over, which is not the same thing and
|
||||||
|
reads as an alignment mistake at every window size.
|
||||||
|
|
||||||
|
The outer width is still `--now-playing-width`, so **the metadata
|
||||||
|
panel's drag handle keeps meaning something**: widening it takes
|
||||||
|
room from the transport on both sides at once, symmetrically. An
|
||||||
|
`1fr … 1fr` pair would have centred the transport just as well and
|
||||||
|
silently made that handle a no-op.
|
||||||
|
|
||||||
|
**The cap is what stops that being a regression**, and it was
|
||||||
|
measured as one first. Reserving the full metadata width on both
|
||||||
|
sides costs the transport twice: at 800px the outer pair wanted
|
||||||
|
640 of 800, and the seek bar's track went from 257px to 61px
|
||||||
|
(and to 0 at 200% text) — the control you drag, squeezed out to
|
||||||
|
centre the buttons above it. So the side tracks are the metadata
|
||||||
|
width *or a quarter of the bar*, whichever is smaller, which
|
||||||
|
leaves the drag handle meaningful everywhere it has room to be
|
||||||
|
and hands the difference to the transport where it does not.
|
||||||
|
|
||||||
|
`minmax(0, …)` on the outer tracks and `min-content` on the middle
|
||||||
|
decide who yields when even that is not enough: the metadata and
|
||||||
|
the end group shrink (both truncate; neither loses an action), and
|
||||||
|
the transport keeps at least its buttons. Without the `min-content`
|
||||||
|
floor the middle collapses first, because a `1fr` track's minimum
|
||||||
|
is `auto` only until something else insists. */
|
||||||
|
--bar-side: min(var(--now-playing-width, 320px), 25%);
|
||||||
|
grid-template-columns:
|
||||||
|
minmax(0, var(--bar-side))
|
||||||
|
minmax(min-content, 1fr)
|
||||||
|
minmax(0, var(--bar-side));
|
||||||
align-items: center;
|
align-items: center;
|
||||||
contain: layout style;
|
contain: layout style;
|
||||||
|
|
||||||
@@ -239,23 +271,44 @@ body div.sidebar {
|
|||||||
text-wrap-mode: nowrap;
|
text-wrap-mode: nowrap;
|
||||||
overflow: hidden;
|
overflow: hidden;
|
||||||
|
|
||||||
p {
|
/* `& p`, not `p`. **A nested rule that begins with a bare
|
||||||
|
element selector is silently dropped before Chrome 120**
|
||||||
|
(relaxed nesting), and the phone this app runs on renders
|
||||||
|
in Chrome 113 -- so this ellipsis, and the two rules
|
||||||
|
below, have never applied on the device. Nothing fails;
|
||||||
|
the text simply overflows there. The `&` form is valid in
|
||||||
|
both, which is why it is used for every element selector
|
||||||
|
in this file's nested blocks. */
|
||||||
|
& p {
|
||||||
overflow: hidden;
|
overflow: hidden;
|
||||||
text-overflow: ellipsis;
|
text-overflow: ellipsis;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
now-playing {
|
& now-playing {
|
||||||
overflow: hidden;
|
overflow: hidden;
|
||||||
}
|
}
|
||||||
|
|
||||||
audio-player {
|
& audio-player {
|
||||||
margin: 0.5em 1em;
|
margin: 0.5em 1em;
|
||||||
|
min-width: 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
/* The right-hand group, and the thing the left column is matched
|
||||||
|
against. It is one grid cell rather than two columns because the
|
||||||
|
centring rule above compares *columns*: volume and the queue
|
||||||
|
button in separate tracks would make the outer pair unequal by
|
||||||
|
whatever the volume happens to measure. */
|
||||||
|
.bar-end {
|
||||||
|
justify-self: end;
|
||||||
|
display: flex;
|
||||||
|
align-items: center;
|
||||||
|
gap: 0.25em;
|
||||||
|
min-width: 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
#queue-button {
|
#queue-button {
|
||||||
justify-self: end;
|
|
||||||
background: none;
|
background: none;
|
||||||
border: none;
|
border: none;
|
||||||
color: inherit;
|
color: inherit;
|
||||||
@@ -440,6 +493,11 @@ body div.sidebar {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@media (max-width: 599px) {
|
@media (max-width: 599px) {
|
||||||
|
/* The phone keeps the two-part bar it had: metadata, then the
|
||||||
|
transport and the queue button. There is no third column to
|
||||||
|
balance because the centring the desktop does is a luxury of
|
||||||
|
having room — at 360px the metadata needs all of the space the
|
||||||
|
controls do not. */
|
||||||
.bottom-bar {
|
.bottom-bar {
|
||||||
grid-template-columns: minmax(0, 1fr) auto auto;
|
grid-template-columns: minmax(0, 1fr) auto auto;
|
||||||
gap: 0.25em;
|
gap: 0.25em;
|
||||||
@@ -448,4 +506,19 @@ body div.sidebar {
|
|||||||
.bottom-bar audio-player {
|
.bottom-bar audio-player {
|
||||||
margin: 0.25em;
|
margin: 0.25em;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Volume stands down here whatever the setting says, because this
|
||||||
|
is about room and about the platform rather than about
|
||||||
|
preference: the hardware keys own volume on a phone, which is
|
||||||
|
also why mediacontrols' Android handler implements no volume
|
||||||
|
callback. It moved from `audio-player`'s own media query when
|
||||||
|
#42 moved the control into the bar — same rule, and now stated
|
||||||
|
where the element actually is.
|
||||||
|
|
||||||
|
`.bottom-bar volume-control`, not the one in
|
||||||
|
`now-playing-view`: that view is the phone's transport and is
|
||||||
|
where a slider does belong. */
|
||||||
|
.bottom-bar volume-control {
|
||||||
|
display: none;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+22
-7
@@ -40,16 +40,31 @@
|
|||||||
</main>
|
</main>
|
||||||
<queue-panel id="queue-panel"></queue-panel>
|
<queue-panel id="queue-panel"></queue-panel>
|
||||||
</div>
|
</div>
|
||||||
|
<!-- Three columns, and the outer two are the same width, which is
|
||||||
|
what makes the middle one *centred* rather than merely in the
|
||||||
|
middle of what is left (#23). The transport used to sit in a
|
||||||
|
`320px 1fr auto` grid, so its centre was ~140px right of the
|
||||||
|
window's.
|
||||||
|
|
||||||
|
That is also why the volume moved out of `audio-player` and
|
||||||
|
into the bar (#42): the transport column has to contain the
|
||||||
|
transport and nothing else, or "centred" means centred with a
|
||||||
|
slider bolted to one side. It joins the queue button in
|
||||||
|
`.bar-end`, whose width is what the left column is matched
|
||||||
|
against. -->
|
||||||
<footer class="bottom-bar">
|
<footer class="bottom-bar">
|
||||||
<now-playing></now-playing>
|
<now-playing></now-playing>
|
||||||
<audio-player></audio-player>
|
<audio-player></audio-player>
|
||||||
<button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
|
<div class="bar-end">
|
||||||
id="queue-button">
|
<volume-control></volume-control>
|
||||||
<!-- ICON_QUEUE in src/utils/icon-language.ts, written out
|
<button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
|
||||||
because this file has no module scope. It was `list`,
|
id="queue-button">
|
||||||
which is the Playlists destination's icon. -->
|
<!-- ICON_QUEUE in src/utils/icon-language.ts, written out
|
||||||
<wa-icon name="bars-staggered"></wa-icon>
|
because this file has no module scope. It was `list`,
|
||||||
</button>
|
which is the Playlists destination's icon. -->
|
||||||
|
<wa-icon name="bars-staggered"></wa-icon>
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
</footer>
|
</footer>
|
||||||
<!-- The phone's primary navigation, hidden above 600px by
|
<!-- The phone's primary navigation, hidden above 600px by
|
||||||
index.css. Eager rather than a chunk, for the reason
|
index.css. Eager rather than a chunk, for the reason
|
||||||
|
|||||||
@@ -18,6 +18,9 @@
|
|||||||
// track-list — index.html renders one, so it is the first paint.
|
// track-list — index.html renders one, so it is the first paint.
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
import '@components/audio-player/audio-player.ts';
|
import '@components/audio-player/audio-player.ts';
|
||||||
|
// In the bar rather than inside `audio-player` since #42, so the shell
|
||||||
|
// is what has to register it.
|
||||||
|
import '@components/audio-player/volume-control/volume-control.ts';
|
||||||
import '@components/track-list/track-list.ts';
|
import '@components/track-list/track-list.ts';
|
||||||
import '@components/now-playing/now-playing.ts';
|
import '@components/now-playing/now-playing.ts';
|
||||||
import '@components/sidebar/app-sidebar.ts';
|
import '@components/sidebar/app-sidebar.ts';
|
||||||
|
|||||||
@@ -3,7 +3,6 @@ import { customElement } from 'lit/decorators.js';
|
|||||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||||
import './controls/player-controls';
|
import './controls/player-controls';
|
||||||
import './seekbar/seek-bar';
|
import './seekbar/seek-bar';
|
||||||
import './volume-control/volume-control';
|
|
||||||
import '../notifications/inline-notice';
|
import '../notifications/inline-notice';
|
||||||
import { PlayerRegion } from '@store/player-store';
|
import { PlayerRegion } from '@store/player-store';
|
||||||
import { designTokens } from '../../styles/tokens.css';
|
import { designTokens } from '../../styles/tokens.css';
|
||||||
@@ -30,6 +29,7 @@ export class AudioPlayer extends LitElement {
|
|||||||
|
|
||||||
.player-main {
|
.player-main {
|
||||||
flex: 1;
|
flex: 1;
|
||||||
|
min-width: 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* The phone transport (plan 016 B2): the buttons, and nothing
|
/* The phone transport (plan 016 B2): the buttons, and nothing
|
||||||
@@ -37,14 +37,17 @@ export class AudioPlayer extends LitElement {
|
|||||||
viewport, not by the host, so this is the component saying what
|
viewport, not by the host, so this is the component saying what
|
||||||
it drops at phone width rather than the shell reaching in.
|
it drops at phone width rather than the shell reaching in.
|
||||||
|
|
||||||
Volume goes because the hardware keys own it on a phone --
|
|
||||||
Android routes them to the media stream, which is also why
|
|
||||||
mediacontrols' Android handler implements no volume callback.
|
|
||||||
The seek bar goes because a 4px-tall target dragged with a thumb
|
The seek bar goes because a 4px-tall target dragged with a thumb
|
||||||
is not a seek control; seeking belongs to the full-screen
|
is not a seek control; seeking belongs to the full-screen
|
||||||
now-playing view, which is the next phase. */
|
now-playing view.
|
||||||
|
|
||||||
|
Volume used to go from here too, and now goes from index.css
|
||||||
|
instead: #42 moved the control out of this component and into
|
||||||
|
the bar, so the shell is what can hide it. The reason is
|
||||||
|
unchanged -- the hardware keys own volume on a phone, which is
|
||||||
|
also why mediacontrols' Android handler implements no volume
|
||||||
|
callback. */
|
||||||
@media (max-width: 599px) {
|
@media (max-width: 599px) {
|
||||||
volume-control,
|
|
||||||
seek-bar {
|
seek-bar {
|
||||||
display: none;
|
display: none;
|
||||||
}
|
}
|
||||||
@@ -65,7 +68,6 @@ export class AudioPlayer extends LitElement {
|
|||||||
<seek-bar></seek-bar>
|
<seek-bar></seek-bar>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<volume-control></volume-control>
|
|
||||||
</div>
|
</div>
|
||||||
`;
|
`;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
|||||||
import '@awesome.me/webawesome/dist/components/slider/slider.js';
|
import '@awesome.me/webawesome/dist/components/slider/slider.js';
|
||||||
import type WaSlider from '@awesome.me/webawesome/dist/components/slider/slider.js';
|
import type WaSlider from '@awesome.me/webawesome/dist/components/slider/slider.js';
|
||||||
import { PlayerController } from '@store/controllers/player-controller';
|
import { PlayerController } from '@store/controllers/player-controller';
|
||||||
|
import { volumeStyleStore } from '@store/volume-style-store';
|
||||||
import { designTokens } from '../../../styles/tokens.css';
|
import { designTokens } from '../../../styles/tokens.css';
|
||||||
import { waSliderLabel } from '../../../styles/wa-slider-label.css';
|
import { waSliderLabel } from '../../../styles/wa-slider-label.css';
|
||||||
|
|
||||||
@@ -22,6 +23,12 @@ export class VolumeControl extends LitElement {
|
|||||||
@state()
|
@state()
|
||||||
private showSlider = false;
|
private showSlider = false;
|
||||||
|
|
||||||
|
/** Whether this is the click-to-open popup rather than a slider. */
|
||||||
|
@state()
|
||||||
|
private popup = volumeStyleStore.popup;
|
||||||
|
|
||||||
|
private unsubscribeStyle?: () => void;
|
||||||
|
|
||||||
// Locally-tracked volume while the user is actively dragging or scrolling.
|
// Locally-tracked volume while the user is actively dragging or scrolling.
|
||||||
// The store's volume only updates once the backend echoes VolumeChanged
|
// The store's volume only updates once the backend echoes VolumeChanged
|
||||||
// (which we debounce), so we track intent here for responsive UI and to let
|
// (which we debounce), so we track intent here for responsive UI and to let
|
||||||
@@ -86,11 +93,31 @@ export class VolumeControl extends LitElement {
|
|||||||
--thumb-height: 16px;
|
--thumb-height: 16px;
|
||||||
}
|
}
|
||||||
|
|
||||||
wa-slider::part(track) {
|
.volume-popup wa-slider::part(track) {
|
||||||
background: var(--yj-text-primary, white);
|
background: var(--yj-text-primary, white);
|
||||||
height: 120px;
|
height: 120px;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* The inline slider (#42). It is the default now, so the width is
|
||||||
|
a real layout decision rather than a detail: 5em is wide enough
|
||||||
|
to aim at and narrow enough that the bottom bar's *outer*
|
||||||
|
columns stay equal without squeezing the transport — which is
|
||||||
|
the arrangement #23 depends on.
|
||||||
|
|
||||||
|
flex-shrink: 0 for the reason the top bar's children have it
|
||||||
|
(#143): a control that quietly gets narrower under pressure
|
||||||
|
hides the fact that the bar has run out of room. This one stands
|
||||||
|
down at phone width instead, in index.css, where the shell can
|
||||||
|
see the viewport. */
|
||||||
|
.inline-slider {
|
||||||
|
width: 5em;
|
||||||
|
flex-shrink: 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
.inline-slider::part(track) {
|
||||||
|
background: var(--yj-text-primary, white);
|
||||||
|
}
|
||||||
|
|
||||||
wa-slider::part(indicator) {
|
wa-slider::part(indicator) {
|
||||||
background: var(--yj-accent, yellow);
|
background: var(--yj-accent, yellow);
|
||||||
}
|
}
|
||||||
@@ -122,8 +149,23 @@ export class VolumeControl extends LitElement {
|
|||||||
// LIFECYCLE
|
// LIFECYCLE
|
||||||
// ===================================================================
|
// ===================================================================
|
||||||
|
|
||||||
|
override connectedCallback() {
|
||||||
|
super.connectedCallback();
|
||||||
|
|
||||||
|
this.unsubscribeStyle = volumeStyleStore.subscribe(() => {
|
||||||
|
this.popup = volumeStyleStore.popup;
|
||||||
|
|
||||||
|
// Switching to the slider while the popup is open would leave the
|
||||||
|
// document listener installed for a popup that no longer renders.
|
||||||
|
if (!this.popup) this.closeSlider();
|
||||||
|
});
|
||||||
|
|
||||||
|
void volumeStyleStore.init();
|
||||||
|
}
|
||||||
|
|
||||||
override disconnectedCallback() {
|
override disconnectedCallback() {
|
||||||
super.disconnectedCallback();
|
super.disconnectedCallback();
|
||||||
|
this.unsubscribeStyle?.();
|
||||||
document.removeEventListener('click', this.boundHandleOutsideClick);
|
document.removeEventListener('click', this.boundHandleOutsideClick);
|
||||||
clearTimeout(this.volumeDebounceTimer);
|
clearTimeout(this.volumeDebounceTimer);
|
||||||
}
|
}
|
||||||
@@ -154,10 +196,12 @@ export class VolumeControl extends LitElement {
|
|||||||
private handleOutsideClick(e: Event) {
|
private handleOutsideClick(e: Event) {
|
||||||
const path = e.composedPath();
|
const path = e.composedPath();
|
||||||
|
|
||||||
if (!path.includes(this)) {
|
if (!path.includes(this)) this.closeSlider();
|
||||||
this.showSlider = false;
|
}
|
||||||
document.removeEventListener('click', this.boundHandleOutsideClick);
|
|
||||||
}
|
private closeSlider() {
|
||||||
|
this.showSlider = false;
|
||||||
|
document.removeEventListener('click', this.boundHandleOutsideClick);
|
||||||
}
|
}
|
||||||
|
|
||||||
private handleInput(e: Event) {
|
private handleInput(e: Event) {
|
||||||
@@ -192,18 +236,46 @@ export class VolumeControl extends LitElement {
|
|||||||
override render() {
|
override render() {
|
||||||
const muted = this.player.muted;
|
const muted = this.player.muted;
|
||||||
|
|
||||||
|
// Inline, the icon is the mute toggle rather than a disclosure:
|
||||||
|
// there is nothing left to disclose, and a button that opens a
|
||||||
|
// popup containing the slider already beside it would be a control
|
||||||
|
// whose only effect is to duplicate its neighbour.
|
||||||
|
const iconAction = this.popup
|
||||||
|
? this.toggleSlider
|
||||||
|
: () => this.player.toggleMute();
|
||||||
|
const iconLabel = this.popup
|
||||||
|
? muted
|
||||||
|
? 'Muted'
|
||||||
|
: `Volume ${this.currentVolume}%`
|
||||||
|
: muted
|
||||||
|
? 'Unmute'
|
||||||
|
: 'Mute';
|
||||||
|
|
||||||
return html`
|
return html`
|
||||||
<button
|
<button
|
||||||
class=${muted ? 'muted' : ''}
|
class=${muted ? 'muted' : ''}
|
||||||
title=${muted ? 'Muted — click for volume' : 'Volume'}
|
title=${muted ? 'Muted — click for volume' : 'Volume'}
|
||||||
aria-label=${muted ? 'Muted' : `Volume ${this.currentVolume}%`}
|
aria-label=${iconLabel}
|
||||||
data-muted=${muted ? 'true' : 'false'}
|
data-muted=${muted ? 'true' : 'false'}
|
||||||
@click="${this.toggleSlider}"
|
@click="${iconAction}"
|
||||||
@wheel="${this.handleWheel}"
|
@wheel="${this.handleWheel}"
|
||||||
>
|
>
|
||||||
<wa-icon name=${this.volumeIcon}></wa-icon>
|
<wa-icon name=${this.volumeIcon}></wa-icon>
|
||||||
</button>
|
</button>
|
||||||
${this.showSlider
|
${!this.popup
|
||||||
|
? html`
|
||||||
|
<wa-slider
|
||||||
|
class="inline-slider ${muted ? 'muted' : ''}"
|
||||||
|
label="Volume"
|
||||||
|
min="0"
|
||||||
|
max="100"
|
||||||
|
.value="${this.currentVolume}"
|
||||||
|
@input="${this.handleInput}"
|
||||||
|
@wheel="${this.handleWheel}"
|
||||||
|
></wa-slider>
|
||||||
|
`
|
||||||
|
: ''}
|
||||||
|
${this.popup && this.showSlider
|
||||||
? html`
|
? html`
|
||||||
<div
|
<div
|
||||||
class="volume-popup ${muted ? 'muted' : ''}"
|
class="volume-popup ${muted ? 'muted' : ''}"
|
||||||
|
|||||||
@@ -25,6 +25,8 @@ import {
|
|||||||
SetQueueFallback,
|
SetQueueFallback,
|
||||||
GetAllowMeteredCatalogDownload,
|
GetAllowMeteredCatalogDownload,
|
||||||
SetAllowMeteredCatalogDownload,
|
SetAllowMeteredCatalogDownload,
|
||||||
|
GetPopupVolume,
|
||||||
|
SetPopupVolume,
|
||||||
} from '@go/config/config.js';
|
} from '@go/config/config.js';
|
||||||
import { GetIndexStatus } from '@go/explore/service.js';
|
import { GetIndexStatus } from '@go/explore/service.js';
|
||||||
import { notificationStore } from '@store/notification-store';
|
import { notificationStore } from '@store/notification-store';
|
||||||
@@ -125,6 +127,8 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
|||||||
@state() private concurrencyMode = 'auto';
|
@state() private concurrencyMode = 'auto';
|
||||||
@state() private defaultPage = 'home';
|
@state() private defaultPage = 'home';
|
||||||
@state() private queueFallback = 'favorites';
|
@state() private queueFallback = 'favorites';
|
||||||
|
|
||||||
|
@state() private popupVolume = false;
|
||||||
@state() private indexStatus: explore.IndexStatus | null = null;
|
@state() private indexStatus: explore.IndexStatus | null = null;
|
||||||
/** Three states, not one: the panel used to say "Loading status…"
|
/** Three states, not one: the panel used to say "Loading status…"
|
||||||
* for the entire session, because the only thing that ever set
|
* for the entire session, because the only thing that ever set
|
||||||
@@ -938,20 +942,28 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
|||||||
|
|
||||||
private async loadLibraries(): Promise<void> {
|
private async loadLibraries(): Promise<void> {
|
||||||
try {
|
try {
|
||||||
const [libs, mode, defaultPage, queueFallback, allowMetered] =
|
const [
|
||||||
await Promise.all([
|
libs,
|
||||||
GetAllLibrariesWithTrackCounts(),
|
mode,
|
||||||
GetScanConcurrency(),
|
defaultPage,
|
||||||
GetDefaultPage(),
|
queueFallback,
|
||||||
GetQueueFallback(),
|
allowMetered,
|
||||||
GetAllowMeteredCatalogDownload(),
|
popupVolume,
|
||||||
]);
|
] = await Promise.all([
|
||||||
|
GetAllLibrariesWithTrackCounts(),
|
||||||
|
GetScanConcurrency(),
|
||||||
|
GetDefaultPage(),
|
||||||
|
GetQueueFallback(),
|
||||||
|
GetAllowMeteredCatalogDownload(),
|
||||||
|
GetPopupVolume(),
|
||||||
|
]);
|
||||||
|
|
||||||
this.libraries = libs ?? [];
|
this.libraries = libs ?? [];
|
||||||
this.concurrencyMode = mode;
|
this.concurrencyMode = mode;
|
||||||
this.defaultPage = defaultPage;
|
this.defaultPage = defaultPage;
|
||||||
this.queueFallback = queueFallback;
|
this.queueFallback = queueFallback;
|
||||||
this.allowMeteredCatalogDownload = allowMetered;
|
this.allowMeteredCatalogDownload = allowMetered;
|
||||||
|
this.popupVolume = popupVolume;
|
||||||
|
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
console.error(
|
console.error(
|
||||||
@@ -1858,10 +1870,51 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
|
|||||||
.value=${this.queueFallback}
|
.value=${this.queueFallback}
|
||||||
@config-change=${this.handleQueueFallbackChange}
|
@config-change=${this.handleQueueFallbackChange}
|
||||||
></config-field>
|
></config-field>
|
||||||
|
<config-field
|
||||||
|
.schema=${{
|
||||||
|
key: 'popupVolume',
|
||||||
|
label: 'Volume opens in a popup',
|
||||||
|
description:
|
||||||
|
'Off, the volume slider is always visible in the '
|
||||||
|
+ 'player bar. On, it hides behind the speaker '
|
||||||
|
+ 'icon. The slider stands down on a phone either '
|
||||||
|
+ 'way, where the hardware keys own volume.',
|
||||||
|
type: 'toggle' as const,
|
||||||
|
}}
|
||||||
|
.value=${this.popupVolume}
|
||||||
|
@config-change=${this.handlePopupVolumeChange}
|
||||||
|
></config-field>
|
||||||
</config-section>
|
</config-section>
|
||||||
`;
|
`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The volume control's presentation (#42).
|
||||||
|
*
|
||||||
|
* In General rather than beside the theme because it is about the
|
||||||
|
* transport's behaviour rather than its colours, and next to "When
|
||||||
|
* the Queue Ends" because both are answers to "how should the
|
||||||
|
* player behave".
|
||||||
|
*/
|
||||||
|
private handlePopupVolumeChange = (
|
||||||
|
e: CustomEvent<ConfigFieldChangeEvent>,
|
||||||
|
): void => {
|
||||||
|
const popup = Boolean(e.detail.value);
|
||||||
|
const previous = this.popupVolume;
|
||||||
|
|
||||||
|
this.popupVolume = popup;
|
||||||
|
|
||||||
|
void SetPopupVolume(popup).catch((err: unknown) => {
|
||||||
|
console.error('failed to save the volume control setting', err);
|
||||||
|
this.popupVolume = previous;
|
||||||
|
notificationStore.transient({
|
||||||
|
key: 'popup-volume-setting',
|
||||||
|
title: 'Setting not saved',
|
||||||
|
text: describeError(err, 'That setting could not be saved.'),
|
||||||
|
});
|
||||||
|
});
|
||||||
|
};
|
||||||
|
|
||||||
// --- Navigation section ---
|
// --- Navigation section ---
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -188,6 +188,27 @@ export class NowPlaying extends LitElement {
|
|||||||
cursor: pointer;
|
cursor: pointer;
|
||||||
/* The art shows through; this is a target, not a picture. */
|
/* The art shows through; this is a target, not a picture. */
|
||||||
color: transparent;
|
color: transparent;
|
||||||
|
/* **Above the art, or it is not a target at all** (#150).
|
||||||
|
|
||||||
|
This button is absolutely positioned with z-index auto and
|
||||||
|
the art is a *later* sibling, so the two tie on paint order
|
||||||
|
and the later one wins. With an <img> that costs nothing --
|
||||||
|
an image is not a hit-test obstacle here -- but a track with
|
||||||
|
no artwork renders a placeholder wa-icon, which is, and it
|
||||||
|
takes every click aimed at the button underneath it.
|
||||||
|
|
||||||
|
The failure is therefore per *track*, not per build: on a
|
||||||
|
phone the only way into the full-screen now-playing view
|
||||||
|
stopped working whenever the current song had no cover.
|
||||||
|
Measured with elementFromPoint at the button's centre --
|
||||||
|
wa-icon with a placeholder, button.expand with an image, and
|
||||||
|
button.expand either way once this line exists.
|
||||||
|
|
||||||
|
z-index rather than pointer-events: none on the art, which
|
||||||
|
would take the cover preview's mouseenter with it; and
|
||||||
|
rather than reordering the DOM, which would leave the same
|
||||||
|
tie to be won by the same accident in the other direction. */
|
||||||
|
z-index: 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
.expand:focus-visible {
|
.expand:focus-visible {
|
||||||
|
|||||||
@@ -0,0 +1,85 @@
|
|||||||
|
import { EventsOn } from '@runtime/runtime';
|
||||||
|
import { GetPopupVolume } from '@go/config/config.js';
|
||||||
|
import { Events } from '../events';
|
||||||
|
|
||||||
|
type Subscriber = () => void;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Whether the volume control is a click-to-open popup (#42).
|
||||||
|
*
|
||||||
|
* The popup was the only option, and "click open, drag, click closed"
|
||||||
|
* is three gestures for a control a bottom bar has room to just show.
|
||||||
|
* So an inline slider is the default and the popup is a setting.
|
||||||
|
*
|
||||||
|
* **The stored flag names the popup, not the slider**, which is the
|
||||||
|
* polarity rule `backend/config` states for every option it has: the
|
||||||
|
* zero value has to be the intended answer. An `InlineVolume bool`
|
||||||
|
* would default to false, hand the popup to every existing install, and
|
||||||
|
* need a migration to say what the default already says.
|
||||||
|
*
|
||||||
|
* It is a store rather than a field on the component because two
|
||||||
|
* components render `<volume-control>` — the bottom bar and the phone's
|
||||||
|
* full-screen now-playing view — and a setting that only reached
|
||||||
|
* whichever one happened to mount after it changed is the fault
|
||||||
|
* `active-view-store` exists to prevent, one surface over.
|
||||||
|
*
|
||||||
|
* The initial value is the *default* rather than a pending answer, so
|
||||||
|
* the first paint is the inline slider and not an empty gap that
|
||||||
|
* becomes one. An install that has chosen the popup sees it swap once
|
||||||
|
* on load, which is the cheaper of the two wrong first frames: the
|
||||||
|
* inline slider occupies the space the popup's button would have.
|
||||||
|
*/
|
||||||
|
class VolumeStyleStore {
|
||||||
|
private value = false;
|
||||||
|
|
||||||
|
private loaded = false;
|
||||||
|
|
||||||
|
private subscribers = new Set<Subscriber>();
|
||||||
|
|
||||||
|
constructor() {
|
||||||
|
EventsOn(Events.GeneralConfigChanged, () => {
|
||||||
|
void this.refresh();
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Whether to draw the popup. Safe to read before `init()`. */
|
||||||
|
get popup(): boolean {
|
||||||
|
return this.value;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Reads the setting once. Safe to call from every mount. */
|
||||||
|
async init(): Promise<void> {
|
||||||
|
if (this.loaded) return;
|
||||||
|
|
||||||
|
this.loaded = true;
|
||||||
|
|
||||||
|
await this.refresh();
|
||||||
|
}
|
||||||
|
|
||||||
|
subscribe(fn: Subscriber): () => void {
|
||||||
|
this.subscribers.add(fn);
|
||||||
|
|
||||||
|
return () => this.subscribers.delete(fn);
|
||||||
|
}
|
||||||
|
|
||||||
|
private async refresh(): Promise<void> {
|
||||||
|
try {
|
||||||
|
const popup = await GetPopupVolume();
|
||||||
|
|
||||||
|
if (popup === this.value) return;
|
||||||
|
|
||||||
|
this.value = popup;
|
||||||
|
this.notify();
|
||||||
|
} catch (err) {
|
||||||
|
// Nothing to tell the user: the control renders in its
|
||||||
|
// default presentation, which is a working volume control.
|
||||||
|
console.error('failed to read the volume control setting', err);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private notify(): void {
|
||||||
|
for (const fn of this.subscribers) fn();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
export const volumeStyleStore = new VolumeStyleStore();
|
||||||
@@ -11,7 +11,7 @@ import '@components/audio-player/controls/player-controls';
|
|||||||
import '@components/audio-player/seekbar/seek-bar';
|
import '@components/audio-player/seekbar/seek-bar';
|
||||||
import '@components/audio-player/volume-control/volume-control';
|
import '@components/audio-player/volume-control/volume-control';
|
||||||
import { Events } from '../../src/events';
|
import { Events } from '../../src/events';
|
||||||
import { emit, calls, lastArgs, flush } from '@test/support/harness';
|
import { emit, calls, lastArgs, flush, stub } from '@test/support/harness';
|
||||||
import {
|
import {
|
||||||
fixture,
|
fixture,
|
||||||
shadow,
|
shadow,
|
||||||
@@ -434,13 +434,40 @@ describe('<seek-bar>', () => {
|
|||||||
* be driven by its own event — watching the volume number, as it used
|
* be driven by its own event — watching the volume number, as it used
|
||||||
* to, meant pressing M visibly did nothing.
|
* to, meant pressing M visibly did nothing.
|
||||||
*/
|
*/
|
||||||
|
/**
|
||||||
|
* The volume control has two presentations (#42), and the icon button
|
||||||
|
* means a different thing in each — so both are exercised rather than
|
||||||
|
* whichever one happens to be the default.
|
||||||
|
*
|
||||||
|
* Inline is the default: the slider is simply there, which leaves the
|
||||||
|
* icon with nothing to disclose, so it is the mute toggle and is named
|
||||||
|
* after that action. In the popup it is a disclosure, so it is named
|
||||||
|
* after the *state* it is showing.
|
||||||
|
*/
|
||||||
describe('volume control: mute', () => {
|
describe('volume control: mute', () => {
|
||||||
beforeEach(() => {
|
/**
|
||||||
|
* Put the presentation back to the default between tests.
|
||||||
|
*
|
||||||
|
* `volumeStyleStore` is a singleton whose `init()` reads the setting
|
||||||
|
* once, so stubbing the binding inside a test is too late — a
|
||||||
|
* previous test has already loaded it. `GeneralConfigChanged` is the
|
||||||
|
* store's own refresh trigger and the same one the Settings page
|
||||||
|
* fires, so driving it that way exercises the real path instead of
|
||||||
|
* reaching for a test-only reset.
|
||||||
|
*/
|
||||||
|
const setPresentation = async (popup: boolean) => {
|
||||||
|
stub('config.Config.GetPopupVolume', popup);
|
||||||
|
emit(Events.GeneralConfigChanged, {});
|
||||||
|
await flush();
|
||||||
|
};
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
await setPresentation(false);
|
||||||
emit(Events.VolumeChanged, 40);
|
emit(Events.VolumeChanged, 40);
|
||||||
emit(Events.MuteChanged, false);
|
emit(Events.MuteChanged, false);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('shows a muted glyph and label once the backend reports mute', async () => {
|
it('shows a muted glyph once the backend reports mute', async () => {
|
||||||
const el = await fixture('volume-control');
|
const el = await fixture('volume-control');
|
||||||
|
|
||||||
expect(shadow(el, 'button')?.getAttribute('data-muted')).toBe('false');
|
expect(shadow(el, 'button')?.getAttribute('data-muted')).toBe('false');
|
||||||
@@ -453,14 +480,53 @@ describe('volume control: mute', () => {
|
|||||||
expect(shadow(el, 'button wa-icon')?.getAttribute('name')).toBe(
|
expect(shadow(el, 'button wa-icon')?.getAttribute('name')).toBe(
|
||||||
'volume-xmark',
|
'volume-xmark',
|
||||||
);
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('names the inline icon after the action it performs', async () => {
|
||||||
|
const el = await fixture('volume-control');
|
||||||
|
|
||||||
|
expect(shadow(el, 'button')?.getAttribute('aria-label')).toBe('Mute');
|
||||||
|
|
||||||
|
emit(Events.MuteChanged, true);
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
expect(shadow(el, 'button')?.getAttribute('aria-label')).toBe('Unmute');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('names the popup icon after the state it discloses', async () => {
|
||||||
|
await setPresentation(true);
|
||||||
|
|
||||||
|
const el = await fixture('volume-control');
|
||||||
|
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
|
expect(shadow(el, 'button')?.getAttribute('aria-label')).toBe(
|
||||||
|
'Volume 40%',
|
||||||
|
);
|
||||||
|
|
||||||
|
emit(Events.MuteChanged, true);
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
|
||||||
expect(shadow(el, 'button')?.getAttribute('aria-label')).toBe('Muted');
|
expect(shadow(el, 'button')?.getAttribute('aria-label')).toBe('Muted');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('shows the slider without a click when it is inline', async () => {
|
||||||
|
const el = await fixture('volume-control');
|
||||||
|
|
||||||
|
// The whole point of the issue: no disclosure to operate first.
|
||||||
|
expect(shadow<HTMLInputElement>(el, 'wa-slider')?.value).toBe(40);
|
||||||
|
});
|
||||||
|
|
||||||
it('keeps showing the volume level while muted, because it is unchanged', async () => {
|
it('keeps showing the volume level while muted, because it is unchanged', async () => {
|
||||||
|
await setPresentation(true);
|
||||||
emit(Events.MuteChanged, true);
|
emit(Events.MuteChanged, true);
|
||||||
await flush();
|
await flush();
|
||||||
|
|
||||||
const el = await fixture('volume-control');
|
const el = await fixture('volume-control');
|
||||||
|
|
||||||
|
await el.updateComplete;
|
||||||
await click(el, 'button');
|
await click(el, 'button');
|
||||||
|
|
||||||
expect(shadow<HTMLInputElement>(el, 'wa-slider')?.value).toBe(40);
|
expect(shadow<HTMLInputElement>(el, 'wa-slider')?.value).toBe(40);
|
||||||
@@ -468,11 +534,24 @@ describe('volume control: mute', () => {
|
|||||||
|
|
||||||
it('toggles mute through the backend rather than locally', async () => {
|
it('toggles mute through the backend rather than locally', async () => {
|
||||||
const el = await fixture('volume-control');
|
const el = await fixture('volume-control');
|
||||||
|
|
||||||
|
await click(el, 'button');
|
||||||
|
|
||||||
|
expect(calls('player.Player.MuteToggle').length).toBe(1);
|
||||||
|
// Nothing optimistic: the icon follows the backend's event.
|
||||||
|
expect(shadow(el, 'button')?.getAttribute('data-muted')).toBe('false');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('toggles mute from inside the popup, where the icon is a disclosure', async () => {
|
||||||
|
await setPresentation(true);
|
||||||
|
|
||||||
|
const el = await fixture('volume-control');
|
||||||
|
|
||||||
|
await el.updateComplete;
|
||||||
await click(el, 'button');
|
await click(el, 'button');
|
||||||
await click(el, '.mute-toggle');
|
await click(el, '.mute-toggle');
|
||||||
|
|
||||||
expect(calls('player.Player.MuteToggle').length).toBe(1);
|
expect(calls('player.Player.MuteToggle').length).toBe(1);
|
||||||
// Nothing optimistic: the icon follows the backend's event.
|
|
||||||
expect(shadow(el, 'button')?.getAttribute('data-muted')).toBe('false');
|
expect(shadow(el, 'button')?.getAttribute('data-muted')).toBe('false');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user