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

Merge pull request 'Centre the transport, and show the volume inline' (#155) from feat/42-inline-volume-and-centred-transport into main
CI / check (push) Successful in 2m28s
Details
CI / e2e (push) Successful in 8m21s
Details

Browse Source 
Three columns whose outer two match, so the middle is centred; the
volume moves into the bar as a slider, with the popup as a setting.

Closes #23
Closes #42
This commit was merged in pull request #155.
This commit is contained in:
logan
2026-08-20 05:22:03 +00:00
parent 86e7444603 dc8db159f9
commit dc6625d33a
17 changed files with 817 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.planning/NOTES.md
+68
View File
@@ -3961,3 +3961,71 @@ 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.
CLAUDE.md
+44
View File
@@ -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,
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
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
backend/config/config.go
+43
View File
@@ -666,6 +666,49 @@ func (c *Config) SetAllowMeteredCatalogDownload(allow bool) error {
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
// show, answered for every known view rather than only the ones the
// config mentions -- so the frontend filters on a value and never has
backend/config/emit_test.go
+40
View File
@@ -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")
}
}
backend/config/general.go
+10
View File
@@ -54,6 +54,16 @@ type GeneralConfig struct {
// so an existing config with no such key refuses by default rather
// than needing a migration to become careful.
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.
e2e/specs/bottom-bar.spec.ts
+147
View File
@@ -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);
});
});
e2e/specs/control-names.spec.ts
+4 -9
View File
@@ -29,18 +29,13 @@ test.describe('a control says what it controls', () => {
});
test('the volume slider is announced as Volume', async ({ app }) => {
// The popup renders no slider at all while closed, the same way the
// queue panel renders no list — so this has to open it first.
await app.getByRole('button', { name: /volume/i }).click();
// No disclosure to open first, and no state to put back afterwards:
// #42 made the slider inline, so it is simply there. The assertion
// is unchanged — the *name* is the subject here, and the route to
// the control got shorter rather than different.
await expect(
app.getByRole('slider', { name: 'Volume' }),
).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 }) => {
e2e/specs/phone-shell.spec.ts
+20 -3
View File
@@ -154,9 +154,26 @@ test.describe('the shell on a phone', () => {
await expect(app.locator('now-playing')).toBeVisible();
// 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
// full-screen now-playing view.
await expect(app.locator('audio-player volume-control')).toBeHidden();
// is not a thumb target -- both belong to the full-screen
// now-playing view.
//
// `.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();
});
});
frontend/bindings/yellowjacket/backend/config/config.ts
+20
View File
@@ -69,6 +69,14 @@ export function GetPinDefaultPlaylist(): $CancellablePromise<boolean> {
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
* runs out.
@@ -207,6 +215,18 @@ export function SetPinDefaultPlaylist(pin: boolean): $CancellablePromise<void> {
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.
*/
frontend/index.css
+78 -5
View File
@@ -211,7 +211,39 @@ body div.sidebar {
padding: 0.25em;
background-color: var(--yj-bg-elevated, #343a40);
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;
contain: layout style;
@@ -239,23 +271,44 @@ body div.sidebar {
text-wrap-mode: nowrap;
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;
text-overflow: ellipsis;
}
}
}
now-playing {
& now-playing {
overflow: hidden;
}
audio-player {
& audio-player {
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 {
justify-self: end;
background: none;
border: none;
color: inherit;
@@ -440,6 +493,11 @@ body div.sidebar {
}
@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 {
grid-template-columns: minmax(0, 1fr) auto auto;
gap: 0.25em;
@@ -448,4 +506,19 @@ body div.sidebar {
.bottom-bar audio-player {
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;
}
}
frontend/index.html
+22 -7
View File
@@ -40,16 +40,31 @@
</main>
<queue-panel id="queue-panel"></queue-panel>
</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">
<now-playing></now-playing>
<audio-player></audio-player>
<button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
id="queue-button">
<!-- ICON_QUEUE in src/utils/icon-language.ts, written out
because this file has no module scope. It was `list`,
which is the Playlists destination's icon. -->
<wa-icon name="bars-staggered"></wa-icon>
</button>
<div class="bar-end">
<volume-control></volume-control>
<button aria-label="Toggle queue" aria-controls="queue-panel" aria-expanded="false"
id="queue-button">
<!-- ICON_QUEUE in src/utils/icon-language.ts, written out
because this file has no module scope. It was `list`,
which is the Playlists destination's icon. -->
<wa-icon name="bars-staggered"></wa-icon>
</button>
</div>
</footer>
<!-- The phone's primary navigation, hidden above 600px by
index.css. Eager rather than a chunk, for the reason
frontend/index.ts
+3
View File
@@ -18,6 +18,9 @@
// track-list — index.html renders one, so it is the first paint.
// ---------------------------------------------------------------------------
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/now-playing/now-playing.ts';
import '@components/sidebar/app-sidebar.ts';
frontend/src/components/audio-player/audio-player.ts
+9 -7
View File
@@ -3,7 +3,6 @@ import { customElement } from 'lit/decorators.js';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import './controls/player-controls';
import './seekbar/seek-bar';
import './volume-control/volume-control';
import '../notifications/inline-notice';
import { PlayerRegion } from '@store/player-store';
import { designTokens } from '../../styles/tokens.css';
@@ -30,6 +29,7 @@ export class AudioPlayer extends LitElement {
.player-main {
flex: 1;
min-width: 0;
}
/* 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
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
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) {
volume-control,
seek-bar {
display: none;
}
@@ -65,7 +68,6 @@ export class AudioPlayer extends LitElement {
<seek-bar></seek-bar>
</div>
</div>
<volume-control></volume-control>
</div>
`;
}
frontend/src/components/audio-player/volume-control/volume-control.ts
+80 -8
View File
@@ -4,6 +4,7 @@ import '@awesome.me/webawesome/dist/components/icon/icon.js';
import '@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 { volumeStyleStore } from '@store/volume-style-store';
import { designTokens } from '../../../styles/tokens.css';
import { waSliderLabel } from '../../../styles/wa-slider-label.css';
@@ -22,6 +23,12 @@ export class VolumeControl extends LitElement {
@state()
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.
// 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
@@ -86,11 +93,31 @@ export class VolumeControl extends LitElement {
--thumb-height: 16px;
}
wa-slider::part(track) {
.volume-popup wa-slider::part(track) {
background: var(--yj-text-primary, white);
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) {
background: var(--yj-accent, yellow);
}
@@ -122,8 +149,23 @@ export class VolumeControl extends LitElement {
// 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() {
super.disconnectedCallback();
this.unsubscribeStyle?.();
document.removeEventListener('click', this.boundHandleOutsideClick);
clearTimeout(this.volumeDebounceTimer);
}
@@ -154,10 +196,12 @@ export class VolumeControl extends LitElement {
private handleOutsideClick(e: Event) {
const path = e.composedPath();
if (!path.includes(this)) {
this.showSlider = false;
document.removeEventListener('click', this.boundHandleOutsideClick);
}
if (!path.includes(this)) this.closeSlider();
}
private closeSlider() {
this.showSlider = false;
document.removeEventListener('click', this.boundHandleOutsideClick);
}
private handleInput(e: Event) {
@@ -192,18 +236,46 @@ export class VolumeControl extends LitElement {
override render() {
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`
<button
class=${muted ? 'muted' : ''}
title=${muted ? 'Muted — click for volume' : 'Volume'}
aria-label=${muted ? 'Muted' : `Volume ${this.currentVolume}%`}
aria-label=${iconLabel}
data-muted=${muted ? 'true' : 'false'}
@click="${this.toggleSlider}"
@click="${iconAction}"
@wheel="${this.handleWheel}"
>
<wa-icon name=${this.volumeIcon}></wa-icon>
</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`
<div
class="volume-popup ${muted ? 'muted' : ''}"
frontend/src/components/config-page/config-page.ts
+61 -8
View File
@@ -25,6 +25,8 @@ import {
SetQueueFallback,
GetAllowMeteredCatalogDownload,
SetAllowMeteredCatalogDownload,
GetPopupVolume,
SetPopupVolume,
} from '@go/config/config.js';
import { GetIndexStatus } from '@go/explore/service.js';
import { notificationStore } from '@store/notification-store';
@@ -125,6 +127,8 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
@state() private concurrencyMode = 'auto';
@state() private defaultPage = 'home';
@state() private queueFallback = 'favorites';
@state() private popupVolume = false;
@state() private indexStatus: explore.IndexStatus | null = null;
/** Three states, not one: the panel used to say "Loading status…"
* 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> {
try {
const [libs, mode, defaultPage, queueFallback, allowMetered] =
await Promise.all([
GetAllLibrariesWithTrackCounts(),
GetScanConcurrency(),
GetDefaultPage(),
GetQueueFallback(),
GetAllowMeteredCatalogDownload(),
]);
const [
libs,
mode,
defaultPage,
queueFallback,
allowMetered,
popupVolume,
] = await Promise.all([
GetAllLibrariesWithTrackCounts(),
GetScanConcurrency(),
GetDefaultPage(),
GetQueueFallback(),
GetAllowMeteredCatalogDownload(),
GetPopupVolume(),
]);
this.libraries = libs ?? [];
this.concurrencyMode = mode;
this.defaultPage = defaultPage;
this.queueFallback = queueFallback;
this.allowMeteredCatalogDownload = allowMetered;
this.popupVolume = popupVolume;
} catch (err) {
console.error(
@@ -1858,10 +1870,51 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
.value=${this.queueFallback}
@config-change=${this.handleQueueFallbackChange}
></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>
`;
}
/**
* 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 ---
/**
frontend/src/store/volume-style-store.ts
+85
View File
@@ -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();
frontend/test/components/transport.test.ts
+83 -4
View File
@@ -11,7 +11,7 @@ import '@components/audio-player/controls/player-controls';
import '@components/audio-player/seekbar/seek-bar';
import '@components/audio-player/volume-control/volume-control';
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 {
fixture,
shadow,
@@ -434,13 +434,40 @@ describe('<seek-bar>', () => {
* be driven by its own event — watching the volume number, as it used
* 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', () => {
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.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');
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(
'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');
});
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 () => {
await setPresentation(true);
emit(Events.MuteChanged, true);
await flush();
const el = await fixture('volume-control');
await el.updateComplete;
await click(el, 'button');
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 () => {
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, '.mute-toggle');
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');
});
});